Skip to content

Feature: rework out_app_flag as a three-mode output controller with unified output directories, append semantics, and provenance headers - #8159

Merged
mohanchen merged 122 commits into
deepmodeling:developfrom
mohanchen:2026-10-16-b
Nov 4, 2026
Merged

mohanchen merged 122 commits into
deepmodeling:developfrom
mohanchen:2026-10-16-b

Conversation

@mohanchen

@mohanchen mohanchen commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This PR reworks out_app_flag from a boolean into a three-mode integer switch
and, on top of that, reorganizes ionic-step outputs into unified directories,
fixes several MPI/MD output bugs, and adds machine-readable provenance headers
to user-facing output files.

out_app_flag semantics (bool -> int)

  • out_app_flag is now an int with three modes:
    • 0 (default): legacy per-step file names and layout (unchanged behavior).
    • 1: fixed file names; per-ionic-step records are appended to a single
      accumulated file (cube frames, PW/LCAO wavefunctions, band output, DOS/PDOS,
      DM(k), T(R)/R(R)/dH/dS sparse matrices, STRU, ...), each record carrying a
      one-based STEP: N provenance header.
    • 2: per-step files routed into unified subdirectories (STRU/ for
      structures, MAT/ for sparse matrices such as dmr/tr/rr/dh/ds/lx, etc.),
      created on demand.
  • INPUT documentation updated to describe the fixed-name behavior of mode 1.

Output renames and provenance headers

  • Orbital information output renamed from Orbital to nao.txt.
  • Band-energy-term outputs renamed (kinetic_out -> eband_kin,
    vhartree_out -> eband_hart, vpp_local_out -> eband_vl,
    vpp_nonlocal_out -> eband_vnl, vxc -> eband_vxc), with per-ionic-step
    provenance headers added to band output.
  • Band-decomposed XC output renamed; LIBRPA producer reference files relocated
    accordingly (manifests updated).
  • LCAO k-space matrix output parameters renamed, and misleading docs fixed.
  • Text LCAO wavefunction output gains a provenance header; the integration
    comparison tool skips comment headers.
  • Ionic-step indices are printed one-based in user-facing outputs (STEP: 1,
    matching the g{N} filename suffix convention), including binary H(R)/S(R)
    and sparse DMR writers; governance doc updated accordingly.

MD / relaxation structure output

  • STRU output directory and naming unified between relax and MD.
  • MD STRU numbering now starts from 1, consistent with relax.
  • md_restartfreq replaced by out_freq_ion + out_stru.
  • Legacy STRU directory layout is preserved for MD structure writing, and the
    MD restart STRU lookup broken by the rework is restored.

Bug fixes

  • Fix MPI crashes in write_dipole when out_dipole=1.
  • Keep all ranks on the FFT path in write_elf (fixes ELF output hangs/crashes).
  • Reuse the fixed dH/dS CSR filename in append mode and log every write.
  • Terminate the DM(k) matrix body with a newline.
  • Route remaining PW outputs through out_app_flag and drop the incorrect
    LCAO-only availability claim.
  • Use the real out_app_flag value for PW wavefunction output; use local
    cached directory variables in ctrl_output_fp.

Tests

  • New three-mode unit tests covering out_app_flag naming and append
    semantics, save_mat overwrite, and write_hsk mode-2 rerun semantics.
  • Sparse-matrix reference files and DMR writer test updated for one-based
    STEP headers; DM(k) test crashes fixed.
  • Integration reference files updated for renamed outputs
    (props_cube.sh, props_mat.sh, LIBRPA manifests, etc.).

Backward compatibility

  • Default behavior (out_app_flag=0) is unchanged; mode 1/2 are opt-in.
  • md_restartfreq is removed in favor of out_freq_ion + out_stru;
    existing inputs using md_restartfreq must migrate.
  • Several output file names changed as listed above; downstream parsers and
    LIBRPA producer consumers should use the updated manifests.

Verification

  • Unit tests: new/updated tests under source/source_io/test,
    source/source_lr, and module test directories (three-mode naming/append,
    sparse writers, STEP headers).
  • Integration tests: updated references in tests/01_PW, 02_NAO_Gamma,
    03_NAO_multik, 08_RI, plus validation tools (props_*.sh,
    check_librpa_producer.py, compare_hsr_binary.py).

abacus_fixer added 8 commits October 16, 2026 14:47
…unified output directories

- Change out_app_flag from bool to int with three states:
  0: no append, no subdirectory (overwrite)
  1: append to same file (default, backward compatible)
  2: write to unified subdirectories
- Add new global directory variables in System_para:
  global_psi_dir, global_chg_dir, global_pot_dir,
  global_elf_dir, global_dos_dir, global_dip_dir, global_mat_dir
- Initialize directories in set_global_dir():
  out_app_flag==2 -> OUT.suffix/{PSI,CHG,POT,ELF,DOS,DIP,MAT}/
  otherwise -> all point to global_out_dir (backward compatible)
- Map legacy WFC->PSI, matrix->MAT
- Update all out_app_flag usages from bool to int semantics
- Broadcast new directory variables via Parallel_Common::bcast_string

Issue: deepmodeling#8151
- ctrl_output_fp.cpp: route chg/tau to global_chg_dir, pot/potes to
  global_pot_dir, ELF to global_elf_dir, dipole to global_dip_dir
- ctrl_output_pw.cpp: route wfc to global_psi_dir, dos to global_dos_dir,
  ldos to global_dos_dir
- ctrl_scf_lcao.cpp: route dos to global_dos_dir
- ctrl_runner_lcao.cpp: route ldos to global_dos_dir
- read_input.cpp: create subdirectories (PSI/CHG/POT/ELF/DOS/DIP/MAT)
  when out_app_flag==2
- Update docs: out_app_flag semantics from bool to int (0/1/2)

Issue: deepmodeling#8151
When out_app_flag defaults to 1 (not 2), all new unified directories
(PSI, CHG, POT, ELF, DOS, DIP, MAT) map to global_out_dir for
backward compatibility. Update io_sys_var_test.cpp expectations:
- global_matrix_dir: "OUT.test/matrix/" -> "OUT.test/"
- Add assertions for all 7 new directory variables
- read_input.cpp: change !out_app_flag to out_app_flag == 0 for
  matrix/WFC legacy directory decisions
- Update three unit tests to use int out_app_flag (0/1) instead of
  bool (false/true), matching the new function signatures:
  test_cal_dos.cpp, test_write_dos_pw.cpp, write_wfc_nao_test.cpp
- Update two comments to describe 0/1 semantics
Replace repeated PARAM.globalv.global_*_dir accesses with the local
const variables already declared at function top, eliminating the
net +4 PARAM global dependency introduced by the path routing change.
With out_app_flag changed from bool to int, the legacy WFC/ subdirectory
is no longer created when out_app_flag=0 (now maps to global_out_dir).
Update the multi-k wfc reference path accordingly.
- read_input.cpp: only create PSI/CHG/POT/ELF/DOS/DIP/MAT subdirectories
  when the corresponding output flag is enabled (out_app_flag=2)
- ctrl_scf_lcao.cpp: route write_dmk and angular momentum calculator
  output to global_mat_dir instead of global_out_dir
- hsr_legacy.cpp: route tr_nao.csr to global_matrix_dir
- pos_op_writer.cpp: route rr_nao.txt to global_matrix_dir
@mohanchen mohanchen added Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes labels Oct 16, 2026
@mohanchen

Copy link
Copy Markdown
Collaborator Author

@ABACUS-PR-BOT please help review the PR

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of 00ece15 with independent verification. Five implementation issues and three non-blocking suggestions are noted inline. Whitespace checks passed; the pinned governance check reported warnings only. No build or runtime tests were run, and CI diagnostics were unavailable.

abacus_fixer and others added 18 commits October 18, 2026 08:10
- Remove md_no_append ternary in hsr_legacy.cpp, pos_op_writer.cpp,
  hterm_writer.cpp, dhs_sparse_writer.cpp: use global_matrix_dir
  unconditionally (it already equals global_out_dir when
  out_app_flag != 2, so no behavior change for legacy modes)
- write_dmr: add dmr_dir parameter, route DM(R) to MAT instead of
  hardcoded global_out_dir; drop parameter.h include
- read_input.cpp: add out_dmr/out_mat_l/out_mat_dh/out_mat_ds to
  MAT subdirectory creation flags

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- global_stru_dir now follows out_app_flag: STRU/ for flag 2,
  global_out_dir otherwise (consistent with MAT/PSI/CHG pattern)
- relax_driver: route STRU_NOW/STRU{step}/STRU_FINAL through
  global_stru_dir instead of global_out_dir
- run_md: drop _MD suffix, use STRU{step} naming consistent with
  relax
- global_file: remove unconditional STRU dir creation from
  make_dir_out; remove global_stru_dir parameter
- read_input: create STRU dir on demand (out_stru>0 or MD)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- run_md: use out_freq_ion (with md_restartfreq fallback) as the
  effective ionic-step output frequency for both STRU and restart
- run_md: gate STRU output on out_stru > 0, consistent with relax
- read_input: simplify STRU dir creation to out_stru > 0
- read_inp_out: update out_stru description to cover MD
- read_input_item_md: mark md_restartfreq as deprecated

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- commit 3a38485 made global_stru_dir follow out_app_flag (default 1
  maps it to the output dir), but the MD restart lookup still probed
  STRU_MD_{istep} there, so restart cases whose fixtures live in
  OUT.suffix/STRU/ (e.g. tests/04_FF/14_LJ_rescale_v) failed to start
- search the current global_stru_dir first, then fall back to the
  legacy OUT.suffix/STRU/ layout; accept both the new STRU{istep+1}
  naming (1-based, unified with relax) and the legacy STRU_MD_{istep}
- quit with a clear diagnostic when no restart structure file exists
- add unit test covering both naming schemes in a user read_file_dir
- add PSI/ candidate in read_wfc_nao so RT-TDDFT restart finds mode-2 snapshots
- route H(k) term outputs to MAT directory; add out_hsr_npz to MAT dir creation
  predicate; narrow DOS dir creation predicate to out_dos/out_ldos
- pass boolean append to save_mat in write_hk_common (was int out_app_flag)
- update out_app_flag/out_wfc_lcao INPUT metadata and position_matrix docs
- add mode-2 directory mapping and PSI layout roundtrip unit tests
Conflict resolutions:
- read_input_test.cpp: keep both added tests (OutAppFlagParsing ours,
  ExplicitNoccSurvivesUnresolvedElectronNumber upstream)
- relax_driver.cpp: identical condition; keep ours comment (md STRU
  output now handled by run_md.cpp, md_restartfreq removed)

Post-merge verification: make -j 30 in build_max_para_test, 100%
Built target abacus_max_para, 0 errors.
In append mode (out_app_flag=1), chg/tau/pot/potes/ELF cube outputs
now reuse a fixed filename (e.g. chgs1.cube, elftot.cube) so every
step overwrites the previous snapshot instead of accumulating
g{N}-suffixed files. Modes 0 and 2 keep the per-step g{N} suffix.
Add an explicit out_app_flag parameter to ModuleIO::write_dmk. In
append mode the geometry-step index is dropped from the filename
(istep_file = -1) so every ionic step overwrites the previous
snapshot, matching the fixed name that read_dmk already expects;
the file body still records the real ionic step.
Drop the g{N} suffix from tr_nao.csr and rr_nao.txt in append mode.
The filenames were changing every ionic step, which defeated the
existing MD append branches in save_sparse and the rr assemble path;
with fixed names those branches now accumulate all steps as designed.
Replace the hardcoded out_app_flag=0 in write_wfc_pw (a 2025-05-17
TODO) with an explicit parameter wired from inp.out_app_flag. In the
default append mode the PW wavefunction files now use fixed names
(wfk1_pw.dat), which finally matches what the restart reader
read_wf2rho_pw looks for; modes 0 and 2 keep the g{N} suffix.
write_chg_init/write_pot_init now keep the fixed _ini.cube names in
append mode so each snapshot overwrites the previous one, instead of
accumulating chgs1g{N}_ini.cube / potg{N}_ini.cube files. These
initial-density cubes are write-only snapshots; no reader consumes
them, so no restart path is affected.
Clarify that mode 1 reuses fixed filenames across steps (text records
append, cube snapshots overwrite), fix the remaining Boolean wording
in out_hsk, and note that out_chg=2/out_pot=3 initial-density cubes
omit the geometry-step index in append mode.
The fixed filename alone still truncated the file every ionic step.
Add a calculation parameter and open the stream with ios::app under
the same condition as open_sparse_file (calculation=="md" &&
out_app_flag==1 && istep>=0), so each MD step appends a new record
while other calculations start fresh.
Rule 22: user-facing output files print ionic steps starting from 1,
matching the g{N} filename suffix convention; zero-based counters such
as "STEP: 0" are not allowed.
Naming (mode 1 fixed name, modes 0/2 keep g{N}):
- DOS (LCAO+PW) and PDOS honor out_app_flag for the g{N} suffix
- lx/ly/lz matrices: Angmom_op::calculate takes out_app_flag

Append (relax now appends like MD; was calculation=="md" only):
- open_sparse_file, rr/dH/dS writers, DM(k): condition relaxed to
  (out_app_flag == 1) && (istep >= 0); SCF passes istep=-1, unaffected
- PDOS and lx/ly/lz append per ionic step in mode 1
- DM(k) drops the calculation parameter again (no longer used)

Step indexing (governance rule 22, one-based):
- STEP: headers of sparse text files and the binary CSR step field
  now start from 1 (hs_sparse_io, pos_op_writer, dhs_sparse_writer,
  hsr_writer)

Build: make -j 30 verified by developer.
Instead of numbered STRU{N} files, out_app_flag=1 now writes one
fixed-name STRU file that accumulates one structure block per written
step, for both relax and MD:

- unitcell::print_stru_file and relax_stru_io::write_stru take a new
  append flag; relax writes "# Ionic Step N" headers (one-based,
  governance rule 22) via the existing build_stru_header
- mdcell::print_stru_file takes frame_comment + append; MD frames are
  prefixed with "# MD step N". The MPI path broadcasts a base offset
  (lseek SEEK_END) and keeps absolute pwrite offsets; O_APPEND is
  deliberately avoided since Linux pwrite ignores the offset with it
- CIF output keeps single-frame semantics: fixed-name STRU.cif is
  overwritten each written step
- modes 0/2 keep the numbered STRU{N} files unchanged
- CLI help of out_stru/out_app_flag documents the mode-1 behavior

Build: make -j 30 verified by developer.
The DM(k) writer emitted the row newline only before each group of
elements, so the file ended without a trailing newline. In append mode
(out_app_flag == 1) the next " --- Ionic Step N ---" record was glued to
the last value of the previous step. Add a trailing newline before
closing the file and cover it with a regression assertion.
abacus_fixer added 10 commits November 1, 2026 16:17
The first int of the native binary CSR record changed meaning from a
zero-based ionic step (v3.11.0-beta9/beta10) to a one-based step without
leaving any marker, so external readers silently misread the step and
shifted every following field. Prefix each record with the magic
0x48535242 ('HSRB') and a format version (2), which keeps the one-based
step while making the layout self-identifying and version-checkable.

Readers now reject a record whose first int is not the magic instead of
guessing the layout. Update the integration comparison tool, the CSR
writer unit tests, the out_hsr help text, and the format documentation
with the migration note.

Verification:
- cmake --build build --target MODULE_IO_hsr_writer_test
- OMP_NUM_THREADS=1 ctest --test-dir build -R MODULE_IO_hsr_writer (2/2 passed)
- cmake --build build --target io_input (compiles read_inp_out.cpp)
- python3 -m py_compile tests/integrate/validation_tools/compare_hsr_binary.py
Mode 1 accumulated one frame per written step, but several writers decided
the append state locally: the relax STRU writer always appended, and the
DM(R)/H(R)/dH callers passed append=true unconditionally and relied on the
leaf writer to re-apply the istep > 0 guard.

Compute the append state at each call site as
out_app_flag == 1 && istep > 0 so a fresh run truncates the fixed-name file
at step 0 and never stacks frames from a previous run, matching the cube,
DM(k) and DOS writers.
The provenance step used by after_all_runners was written from both
before_scf and the TDDFT runner. Record it in the three-argument
before_scf overload instead, so every LCAO entry path funnels through
one write while still passing the output index explicitly.
…iters

write_bands, write_wfc_nao and vxc_op_tools each carried a local copy of
the version/timestamp header builder. Both lines now come from
ModuleIO::provenance_header() and ModuleIO::local_timestamp() in
module_output/provenance_header.h; each writer keeps only its own step
line. local_time_stamp() is removed in favor of local_timestamp().
Header text is unchanged, keeping bands*.txt, wfk*_nao.txt and
eband_*.txt formats compatible.
Replace the hand-maintained `out_app_flag == 2` subdir predicate in
`ReadInput::create_directory` with a lazy `ModuleBase::ensure_directory`
call at each output site. The old predicate had to enumerate every
writer's flags by hand and was patched in three separate review rounds;
moving creation to the writers removes that whole class of drift.

Each ensure sits inside the existing output flag guard, so only
directories that actually receive a file are created. Writers that read
`PARAM.globalv.*_dir` directly (write_dh_perI, write_wfc_nao,
output_mat_sparse) ensure in the leaf; dispatcher-level sites hoist the
directory into a local so the ensure and the write share one lookup.
read_set_globalv.cpp calls stru_frame_io::extract_last_frame through the
io_input object library, but abacus_input_yaml listed neither the source
nor the cell library, so the documentation executable failed to link.
Add the source file to that target, matching the INPUT test targets.
…olation

The boundary-shift propagation restore (6cf34b9) derives
positions_updated from the global MD step (step_ + step_rst_), so a
restarted run's first force evaluation wrongly marks positions as
updated and immediately advances the charge-extrapolation history
with a degenerate state, shifting the PW md_nvt trajectory.

Pass an explicit positions_updated flag into MD_func::force_virial
instead: false for the run's initial evaluation (including restarts),
true after the first MD drift. Add a restart regression unit test and
regenerate tests/01_PW/md_nvt/result.ref.
Resolve the add/add conflict in read_input_item_test.cpp: both sides
appended new InputTest cases at the same location. Keep the branch's
OutMatComponentPrecision / OutStruCifRejectedForMd tests and the
upstream DfptMixType / DfptKerkerA2 / DfptBroydenNdim domain tests.
The out_app_flag 0/1/2 rework made mode 0 (overwrite fixed filename)
the default, so the two MD cases without an explicit out_app_flag now
keep only the last ionic step in hk_nao.txt / hks1_nao.txt, while
their references hold two frames. Restore the historical append
behavior explicitly to keep the multi-step accumulation coverage.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static incremental review at ca7db6e2 found a new MPI STRU write-offset race. The feedback also covers remaining output-retention and metadata gaps, plus newly added test fixtures. No builds or runtime tests were run.

Comment thread source/source_cell/print_cell.cpp Resolved
abacus_fixer added 4 commits November 2, 2026 19:18
Recent bug fixes shifted the converged energies of the two remaining
active LCAO cases; 17_DS_DFTU has never been wired into CI/CD, so the
references are updated manually for now. The folder will be moved and
integrated elsewhere later.

- CASES_CPU.txt: keep only 01_LCAO_SPIN_S2_Z and 03_LCAO_DFTU_S2_Z
- INPUT: tighten scf_thr from 1e-6 to 1e-8 in both cases
- result.ref: refresh etot/etotperatom (shifts of ~1.6e-5 eV and
  ~1.0e-4 eV) generated with the tightened threshold

Verified: Autotest.sh -a build_max_para_test/abacus_max_para -n 4 -o 1
passes both cases (v3.11.0-beta12, ca7db6e).
The MD restart inputs (Restart_md.txt, STRU_MD_10) were force-tracked
inside OUT.autotest/, where test-cleanup scripts delete OUT.* without
distinguishing tracked inputs from run artifacts.

- git mv both files to the case directory top level
- INPUT: set read_file_dir ./ so the restart reader looks up
  Restart_md.txt and STRU_MD_{step} directly in the case directory
  (user-provided read_file_dir bypasses the OUT.{suffix}/STRU search)

Verified: Autotest.sh -a build_max_para_test/abacus_max_para -n 4 -o 1
-r 14_LJ_rescale_v passes all 4 checks (etot/etotperatom/force/stress);
energies identical to the reference, confirming the restart from step 10.
mdcell::print_stru_file had a data race on the first species block offset.
After the MPI_Allreduce that checked per-rank open() success, rank 0 could
proceed to write the species header (pwrite) before another rank had
performed its local lseek(SEEK_END). That late rank would then observe a
larger EOF and compute an offset shifted by the header size, causing all
its atom records to land at the wrong positions and silently corrupt the
restart STRU file. The per-species MPI_Barrier and MPI_Exscan protected
all species after the first, so only the first offset was affected.

Fix: rank 0 computes offset = base_offset + header.size() while preparing
the header (it already knows base_offset from its own SEEK_END), and
broadcasts that single authoritative value to the cell communicator via
Parallel_Common::bcast_mpi_offset, a new protected wrapper added alongside
the existing bcast_* helpers. All ranks then open the file with their
offsets already agreed, removing the dependency on observing the same
filesystem EOF.

Side fix: the previous lseek fallback header.size() ignored base_offset in
append mode; broadcasting the summed value eliminates that inconsistency
as well.

New test: ParallelCommon.BcastMPIOffsetOnCommunicator splits
MPI_COMM_WORLD into two color sub-communicators and verifies that each
sub-communicator's root is the source of the broadcast, not world rank 0
— which is exactly what the cell-communicator use case needs.

Verified with build_max_para_test:
- All 69 MODULE_CELL_* and MODULE_MD_* ctests pass (including
  mdcell_reader_np4, print_cell_test, MD_base_mpi_np4).
- 4-rank restart run of 14_LJ_rescale_v with out_stru out_freq_ion=1
  out_app_flag=1 produces an appended 5-frame STRU where every frame
  has exactly 32 atoms and no overlapping writes.
The md_nvt stress reference (380.833478) was generated by commit
8846951 on a different build/library configuration. The current
build_max_para_test deterministically produces 380.832378, a 1.1e-3 kbar
deviation that just exceeds the 1e-3 kbar stress threshold.

Energy (4.9e-9 eV) and force (1.8e-5) also shift by small amounts in the
same direction, confirming this is a BLAS/LAPACK/FFTW numerical-environment
difference rather than a physical regression. Update the reference to match
the current build so the case passes cleanly.

@Critsium-xy Critsium-xy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of 94e5a2d (no build or run). Six items inline: two regressions introduced by this PR (solver step index on MD restart, restart STRU cadence) and four documentation/compatibility items.

Where an item reflects an intentional design choice, that choice must be stated explicitly in the PR description, not only in code comments or INPUT help text.

Comment thread source/source_md/md_base.cpp Resolved
Comment thread source/source_md/run_md.cpp Resolved

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static recheck of 94e5a2d4: the shared MD structure-offset fix is present. One non-blocking test-selection suggestion remains below. Numerical reference changes were not independently rerun.

Comment thread tests/17_DS_DFTU/CASES_CPU.txt Resolved
@ABACUS-PR-BOT

Copy link
Copy Markdown
Collaborator

Resolved the seven conflicts with develop in e20431c7b, preserving the IO linkage and collector changes. Ten affected targets built; 17 CTest entries, 9 cube tests, 32 collector fixtures, shell syntax checks and YAML generation passed. This verification covers the conflict resolution, not the refreshed numerical references or deferred restart behavior. CI has not been verified.

@AsTonyshment AsTonyshment left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the implementation and verified the relevant tests and output modes, found no new issues affecting physical correctness. LGTM if other comments are addressed.

abacus_fixer and others added 7 commits November 4, 2026 15:16
MD passed the global step (step_ + step_rst_) to ESolver::runner, so a
restarted run's first force evaluation arrived with istep > 0 and the
solver skipped its istep == 0 initialization branches (LCAO init_chg
dm/hr and init_wfc file reads, PW need_subspace special-casing). Pass
the local step_ instead; the global step remains in use only by the
MD-side writers (print_md, dump_info, STRU naming, write_restart,
PLUMED). Extend the restart unit test to assert the solver observes
istep == 0.

Verification: ctest -R MODULE_MD 14/14 passed; tests/01_PW/md_nvt
(restart case) rerun, all deviations within tolerance against the
existing reference (etot 1.2e-8 < 1e-7, stress 3.9e-4 < 1e-3).
         md_restartfreq step

The earlier decouple-cadences change (0d77820) moved Restart_md.txt onto
md_restartfreq independently of out_freq_ion, but left structure frames on
the out_freq_ion cadence gated by out_stru > 0. When the two cadences
disagree (e.g. out_freq_ion=3, md_restartfreq=5) or out_stru=0, the restart
file pointed at an istep for which no STRU frame existed:

  - modes 0/2 (numbered STRU files) hard-failed with no STRU to read,
  - mode 1 (cumulative STRU) silently fell back to the last frame of the
    file without validating its step marker, mixing geometry from one step
    with Restart_md.txt thermostat/step metadata from another step.

Changes:

  * run_md: frames now written whenever either cadence fires; the restart
    checkpoint frame is always written (out_stru gate does not apply), so
    every md_restartfreq step produces a STRU frame paired with its
    Restart_md.txt. A single write covers the joint-step case where both
    cadences coincide, avoiding duplicates in cumulative STRU mode.
    Extracted is_frame_write_step() predicate for unit testing.
  * stru_frame_io: added extract_frame_by_step(cumulative, step, out)
    which walks the cumulative STRU and extracts the frame whose "# MD step
    N" marker exactly matches the requested step.
  * find_md_restart_stru: mode-1 path now uses extract_frame_by_step on
    marker istep+1 instead of unconditionally taking the last frame; the
    unconditional last-frame fallback is removed entirely.
  * Tests: 3 new stru_frame_io tests for extract_frame_by_step and 6 new
    run_md tests covering the "every restart step yields a frame"
    invariant, out_stru=0 behavior, cadence-union semantics, default
    fallback, joint-step dedup, and fully-disabled frequencies.

Verification: MODULE_CELL_stru_frame_io 9/9, MODULE_MD_run 10/10,
MODULE_IO_read_input_serial 13/13, MODULE_MD_{fire,verlet,nhc,langevin,
msst,func} 33/33, mdcell_reader 2/2 (mpirun -n 4) — all passed. Agent
governance check emits only pre-existing warnings (net_global_delta=-8).
… default

本分支把 out_app_flag 从 bool 重写为 0/1/2 三模式整数后,PW 与 LCAO
的 DOS 写文件共用了 "if(istep>=0 && out_app_flag==2)" 守卫——默认
out_app_flag=0 写固定名 doss1.txt,仅模式 2 才追加 g{istep+1} 后缀。
develop 上因为旧逻辑总是追加 g 后缀,examples/13_dos/01_pw_Al 与
02_lcao_Si2 的 run.sh 都检查 doss1g1.txt,且 INPUT2 均未覆盖
out_app_flag,因此在本分支上会"计算成功但脚本判失败"。

同步修复:
- examples/13_dos/{01_pw_Al,02_lcao_Si2}/run.sh:
  OUT.ABACUS/doss1g1.txt -> OUT.ABACUS/doss1.txt
- examples/13_dos/README: 重写命名段落,清楚列出 out_app_flag=0/1/2
  三种模式下的文件名与子目录位置
- docs/advanced/output_files/output-specification.md:
  样例表中的 doss1g1.txt -> doss1.txt,对齐已提交的
  tests/03_NAO_multik/scf_out_dos_spin2/OUT.autotest/ 实际产物
- tests/performance/catch_properties.sh: 硬编码 doss1g1.txt ->
  glob doss*.txt,与 integrate 的 props_basic.sh 策略一致,防止
  以后有人给性能用例开 out_dos 时再次踩坑

验证:
- git diff --check (5 个文件) 无空白错误
- ls tests/03_NAO_multik/scf_out_dos_spin2/OUT.autotest/doss*.txt
  -> doss1.txt, doss2.txt,与文档修改后一致
- 未跑 examples/13_dos 运行时用例:它会触发两轮 mpirun,本地
  sandbox 里 OpenMPI 会有 socket/ifinit 警告,按仓库规则需在
  sandbox 外重跑
…ep} reader

md_restart's TDDFT restart guidance still told users to set out_app_flag=0
and referred to a WFC_NAO_K${kpoint} filename placeholder. The out_app_flag
rework (89aaa48) moved the g{step} suffix exclusively to mode 2, and the
reader for istep>0 rejects fixed-name mode-0/1 files because their single
frame would silently be step 0. Update the description to out_app_flag=2
with the real path (OUT.{suffix}/PSI/wf{k}{s}g{step}_nao.txt) and note why
modes 0/1 are unacceptable for restart.

Alongside, refine the out_app_flag description to clarify:
- mode 0 still truncates fixed-name files (no g{step} since the rework)
- mode 2's STRUC/ subdir holds STRU{istep} frames, parallel to WFC g{step}
- legacy boolean mappings no longer reproduce the old per-step behavior;
  use mode 2 instead.
run_md tags cumulative STRU frames with current_step + 1 while
Restart_md.txt records the 0-based completed-step count (current_step).
Commit 0bbb7c5 made find_md_restart_stru match the frame marker of
istep + 1 exactly and removed the unconditional last-frame fallback, but
the SetGlobalDirMdRestart fixture still wrote Restart_md.txt = 2 with
only "# MD step 1/2" frames, so WARNING_QUIT aborted the test.

Add the "# MD step 3" frame that a real restart from step 2 produces and
document the istep + 1 pairing.

Verification: cmake --build build --target MODULE_IO_system_variable_test;
ctest -R "MODULE_IO_read_input_serial|MODULE_IO_system_variable_test"
passed 2/2.
@mohanchen
mohanchen merged commit 1b0f905 into deepmodeling:develop Nov 4, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes Solve before 3.11

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants